v0.0.1 adapter for non-merklized onchain issuer #284

ilya-korotya · 2024-11-12T13:17:25Z

No description provided.

…verter

package.json

vmidyllic · 2024-11-18T15:56:20Z

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

+import { getDateFromUnixTimestamp } from '@iden3/js-iden3-core';
+import { Options } from '@iden3/js-jsonld-merklization';
+
+const interfaceDetection = '0x01ffc9a7';


let's do it as enum, maybe it makes sence to extend existing one and move to common file

see export enum FunctionSignatures

My main idea was not to export this class to the user. The entry point to the onchain issuer should be only one OnchainIssuer class that decides on the type of OnchainIssuer and creates an instance of the required adapter. If we consider this adapter as some kind of hidden functionality, then it makes no sense to put these variables in the public enum

Moved to enum, but not to common enum: FunctionSignatures

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

vmidyllic · 2024-11-18T16:02:10Z

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

+   * @param merklizationOptions Optional settings for merklization.
+   */
+  constructor(
+    url: string,


should we pass eth config here, so it will be possible to init this storage separately and still connect to contract?

otherwise rename to rpcUrl.
Also leave a possibility to options object
e.g.

new Adapter(address, options: { configs: EthConfigurationConfig[] / rpcUrl : .. , merklizationOptions, issuerDID }

This adapter will not be exported to the end user. The user should use the OnchainIssuer class as an entry point to determine which adapter should be used.

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

...ockchain/onchain-issuer-adapter/non-merklized/version/v0.0.1/types/NonMerklizedIssuerBase.ts

src/storage/blockchain/onchain-issuer.ts

vmidyllic · 2024-11-18T16:20:48Z

tests/adapter/testdata/golang_http_schema.json

@@ -0,0 +1,56 @@
+{


you put a lot of testdata here.. is it really needed?

vmidyllic

why this file is not in abi folder

volodymyr-basiuk · 2024-11-18T18:04:28Z

...ockchain/onchain-issuer-adapter/non-merklized/version/v0.0.1/types/NonMerklizedIssuerBase.ts

+} from './common';
+
+export declare namespace SmtLib {
+  export type ProofStruct = {


we have such struct - StateProof

volodymyr-basiuk · 2024-11-18T18:06:15Z

...ockchain/onchain-issuer-adapter/non-merklized/version/v0.0.1/types/NonMerklizedIssuerBase.ts

+    auxExistence: boolean,
+    auxIndex: bigint,
+    auxValue: bigint
+  ] & {


& StateProof

volodymyr-basiuk · 2024-11-18T18:11:31Z

...ockchain/onchain-issuer-adapter/non-merklized/version/v0.0.1/types/NonMerklizedIssuerBase.ts

@@ -0,0 +1,617 @@
+/* Autogenerated file. Do not edit manually. */


Do we really need this autogenerated file in sdk ?

I suggest removing all autogenerated files and keeping only the .abi files with functions that are actively used in our services. This will simplify the PR, making it easier to review, and ensure alignment with our standard approach to interacting with Smart Contracts.

Yes. We need types for the onchain issuer(s) because it is difficult to manage inline structures, arrays with inline structures, etc.
We decided to move the automatically generated code to https://github.com/iden3/contracts-abi/tree/main/onchain-non-merklized-issuer-base/v0/js and reuse it.

…verter

volodymyr-basiuk · 2024-11-21T11:13:30Z

src/credentials/credential-wallet.ts

-      schema.$metadata.uris['jsonLdContext']
-    ];
+    r.context.push(schema.$metadata.uris['jsonLdContext']);
+    r.expiration = !r.expiration || r.expiration == 0 ? undefined : r.expiration * 1000;


Suggested change

r.expiration = !r.expiration || r.expiration == 0 ? undefined : r.expiration * 1000;

r.expiration = r.expiration ? r.expiration * 1000 : undefined;

I don't see it's done

volodymyr-basiuk · 2024-11-21T11:14:30Z

src/credentials/credential-wallet.ts

-    ];
+    r.context.push(schema.$metadata.uris['jsonLdContext']);
+    r.expiration = !r.expiration || r.expiration == 0 ? undefined : r.expiration * 1000;
+    r.id = r.id ? r.id : `urn:${uuid.v4()}`;


Suggested change

r.id = r.id ? r.id : `urn:${uuid.v4()}`;

r.id = r.id ?? `urn:${uuid.v4()}`;

I tested this on a browser console and there is a result:

const foo = ""; foo ?? console.log(1); returns: '' - empty string const foo = ""; foo ? console.log(2) : console.log(1); returns 1;

I expect with user pass empty string for the id field, we need generate uuid.

In that case replace ?? to || for example a = ''; b = '1'; a = a || b; // '1'

src/credentials/credential-wallet.ts

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

Kolezhniuk · 2024-11-21T13:31:00Z

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

+    if (onchainDisplayMethod.id === '' && onchainDisplayMethod._type === '') {
+      return undefined;
+    }
+    switch (onchainDisplayMethod._type) {


add default behavior to switch clause

I don't see any changes that you say done that they are really done

package.json

vmidyllic · 2024-11-25T08:47:44Z

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

+      merklizationOptions?: Options;
+    }
+  ) {
+    const rpcProvider = new ethers.JsonRpcProvider(options.rpcUrl);


for now I don't see how it would be possible to use 'did resolver' approach on client with this adapter.
We need to think and implement the fetch of onchain credentials through did resolver too.

vmidyllic · 2024-11-25T08:50:06Z

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

+    if (onchainDisplayMethod.id === '' && onchainDisplayMethod._type === '') {
+      return undefined;
+    }
+    switch (onchainDisplayMethod._type) {


I don't see any changes that you say done that they are really done

vmidyllic · 2024-11-25T08:54:09Z

.../onchain-issuer-adapter/non-merklized/version/v0.0.1/onchain-non-merklized-issuer-adapter.ts

+  }
+
+  private credentialId(id: bigint): string {
+    return `urn:iden3:onchain:${this._chainId}:${this._contractAddress}:${id}`;


is urn:iden3:onchain: agreed somewhere?

I'm not sure. I can't find a place where we made a commitment to this format. But Golang lib uses the same format. I think we agreed this format before.

vmidyllic · 2024-11-25T08:55:06Z

fix warnings in linter

ilya-korotya · 2024-11-25T10:57:23Z

The github doesn't represent latest commit.

v0.0.1 adapter for non-merklized onchain issuer

e668d12

ilya-korotya requested review from Kolezhniuk and vmidyllic November 12, 2024 13:17

ilya-korotya added 8 commits November 12, 2024 14:21

Merge branch 'main' into feature/onchain-non-merklized-credential-con…

763a9c6

…verter

fix prettier

117742a

update package-lock

fd10979

add ts-ignore for tests data

4203f40

use internal import

d19e191

use ipfs node url instead of gateway

9da8763

add docs

81ab215

fix prettier

b68392c

Kolezhniuk requested a review from volodymyr-basiuk November 12, 2024 16:34

ilya-korotya added 3 commits November 18, 2024 13:18

move onchain-issuer package on storage level

33ad9b9

fix linter

f8a433b

use ipfs node url instead of gateway

32592ac

vmidyllic requested changes Nov 18, 2024

View reviewed changes

vmidyllic reviewed Nov 18, 2024

View reviewed changes

volodymyr-basiuk reviewed Nov 18, 2024

View reviewed changes

fix comments

01dfd75

ilya-korotya requested review from vmidyllic and volodymyr-basiuk November 20, 2024 16:12

ilya-korotya added 6 commits November 20, 2024 17:18

use node instead of gateway

241c45c

fix package.json

d9f89a4

fix json representation

6fe7eb2

fix prettier

eb9e809

fix expiration time

9ab97d2

avoid mutation

1b7ed93

ilya-korotya force-pushed the feature/onchain-non-merklized-credential-converter branch from 2fe0eb5 to 1b7ed93 Compare November 21, 2024 08:22

ilya-korotya added 3 commits November 21, 2024 10:04

Merge branch 'main' into feature/onchain-non-merklized-credential-con…

c35fa83

…verter

fix package-lock

aae225d

use beta version for nock

e0a18ff

volodymyr-basiuk reviewed Nov 21, 2024

View reviewed changes

Kolezhniuk requested changes Nov 21, 2024

View reviewed changes

Kolezhniuk approved these changes Nov 22, 2024

View reviewed changes

ilya-korotya requested a review from volodymyr-basiuk November 22, 2024 12:08

volodymyr-basiuk approved these changes Nov 22, 2024

View reviewed changes

vmidyllic reviewed Nov 25, 2024

View reviewed changes

ilya-korotya closed this Nov 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.0.1 adapter for non-merklized onchain issuer #284

v0.0.1 adapter for non-merklized onchain issuer #284

ilya-korotya commented Nov 12, 2024

vmidyllic Nov 18, 2024

ilya-korotya Nov 19, 2024

ilya-korotya Nov 20, 2024

vmidyllic Nov 18, 2024 •

edited

Loading

ilya-korotya Nov 20, 2024

vmidyllic Nov 18, 2024

vmidyllic left a comment

volodymyr-basiuk Nov 18, 2024

volodymyr-basiuk Nov 18, 2024

volodymyr-basiuk Nov 18, 2024

volodymyr-basiuk Nov 18, 2024

ilya-korotya Nov 20, 2024

volodymyr-basiuk Nov 21, 2024

ilya-korotya Nov 22, 2024

vmidyllic Nov 25, 2024

volodymyr-basiuk Nov 21, 2024

ilya-korotya Nov 22, 2024

Kolezhniuk Nov 22, 2024

Kolezhniuk Nov 21, 2024

ilya-korotya Nov 22, 2024

vmidyllic Nov 25, 2024

vmidyllic Nov 25, 2024

vmidyllic Nov 25, 2024

vmidyllic Nov 25, 2024

ilya-korotya Nov 25, 2024

vmidyllic commented Nov 25, 2024

ilya-korotya commented Nov 25, 2024

		@@ -0,0 +1,617 @@
		/* Autogenerated file. Do not edit manually. */

	r.expiration = !r.expiration \|\| r.expiration == 0 ? undefined : r.expiration * 1000;
	r.expiration = r.expiration ? r.expiration * 1000 : undefined;

	r.id = r.id ? r.id : `urn:${uuid.v4()}`;
	r.id = r.id ?? `urn:${uuid.v4()}`;

v0.0.1 adapter for non-merklized onchain issuer #284

v0.0.1 adapter for non-merklized onchain issuer #284

Conversation

ilya-korotya commented Nov 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmidyllic Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmidyllic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmidyllic commented Nov 25, 2024

ilya-korotya commented Nov 25, 2024

vmidyllic Nov 18, 2024 •

edited

Loading